-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NoDAG prototyping #1011
base: main
Are you sure you want to change the base?
NoDAG prototyping #1011
Conversation
|
||
func RunWorkflow(runtime sdk.RuntimeV2, triggerOutputs basictrigger.TriggerOutputs) error { | ||
// two action calls "futures" | ||
actionCall1, _ := wasm.NewCapabilityCall[basicaction.ActionInputs, basicaction.ActionConfig, basicaction.ActionOutputs]( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two things. First, the runtime should make the call, or you can't test without WASM. Second, we'll have generated helpers for the calls that take and return the right types given a runtime :).
pkg/workflows/wasm/runner_v2.go
Outdated
switch { | ||
case req.GetSpecRequest() != nil: | ||
rsp, innerErr := r.handleSpecRequest(factory, req.Id) | ||
if innerErr != nil { | ||
resp.ErrMsg = innerErr.Error() | ||
} else { | ||
resp = rsp | ||
} | ||
case req.GetRunRequest() != nil: | ||
rsp, innerErr := r.handleRunRequest(factory, req.Id, req.GetRunRequest()) | ||
if innerErr != nil { | ||
resp.ErrMsg = innerErr.Error() | ||
} else { | ||
resp = rsp // should happen only when workflow is done processing (i.e. no more capability calls) | ||
} | ||
default: | ||
resp.ErrMsg = "invalid request: message must be SpecRequest or RunRequest" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For regular execution, I don't think the WASM should asked to do work. How would I enable parallelism?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite follow but I think the planned change to use a separate import will resolve this concern.
No description provided.